Note: This PR will need to be released and have at least one daily release run to get the changes into the aws-sdk-s3 crate before the feature/http-1.x branch can be successfully merged to main.

Motivation and Context

When merging main back to the feature/http-1.x branch in #4422 we started to see a new failure in the semver hazards test.

failures:

---- aws_chunked::tests::test_aws_chunked_body_is_retryable stdout ----

thread 'aws_chunked::tests::test_aws_chunked_body_is_retryable' panicked at sdk/s3/src/aws_chunked.rs:195:47:
called `Result::unwrap()` on an `Err` value: "Trailers polled while body still has data"

The semver hazards test works by checking out the published versions of the SDK crates in the aws-sdk-rust repo, patching those crates to use the PRed versions of the runtime crates, and running the tests in the SDK crates with those patches in place.

The currently published version of test_aws_chunked_body_is_retryable creates a temp file with size 10000 x line.len(). The AwsChunkedBodyOptions are created with ::default() which sets stream_length = 0 . That means we hit this check in the http_body_04x::Body impl for AwsChunkedBody which sets the state to AwsChunkedBodyState::WritingTrailers without ever polling the inner body. The next time we poll we call poll_trailers on the inner body it ends up at the "Trailers polled while body still has data" error. This error is correct, we are polling trailers while the body still has data.

It turns out Yuki already did a fix for this in his merge from main to the feature branch by setting the stream_length and changing the asserts to properly look for the body data. That change is in an inlineable and still in the feature branch, so the published version of the S3 crate used by the semver hazards check doesn't have the fixed test.

The minimum fix here is to port the changes for the tests in aws_chunked to main since the current version of those tests is kind of wrong anyway, not polling through the body data at all. We got (un)lucky this showed up in the feature branch because the http-1x SdkBody checks for body data when polling trailers since they can't be polled separately.

But all of this debugging brings us to the underlying bug, when the inner body of an SdkBody is BoxBody::HttpBody04 our implementation will happily let you poll trailers while the body still has data. As far as I am aware there is no use case that would make this a reasonable thing to do. To fix the actual bug I see two possible solutions:

• We could fix the underlying bug by updating the BoxBody::HttpBody04 branch in SdkBody's poll_next_trailer method to return an error by calling poll_data on the inner body in poll_next_trailers and failing if it returns anything other than Poll::Ready(none). Unfortunately it seems possible that this could cause tricky runtime breakages for existing users, so probably isn't a tenable option.
• Update the BoxBody::HttpBody1 branch in SdkBody's poll_next_trailer method to act more like the BoxBody::HttpBody04 branch. In this case if it encountered a data frame rather than a trailer frame it would keep polling the frames until the body data is exhausted (either buffering or just throwing away the data frames) and we get a trailer frame or run out of frames. This feels incorrect as polling the trailers before the body is exhausted shouldn't ever be correct, and polling the trailer in HttpBody04 doesn't actually throw away the body bytes, they could still be polled later.

Testing

I used runtime-versioner patch-runtime locally to patch the feature/http-1.x branch versions of the runtime crates into a local checkout of theaws-sdk-s3 crate. This reproduced the failure locally. Manually updating the inlineable aws_chunked.rs in the S3 crate with the changes from this PR allowed the tests to pass.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.